Skip to content

Conversation

@jorgenfj
Copy link
Contributor

Experimented with new structure for utils.

Adds more boilerplate but should be more scalable.

By simply defining some acessors for your type you can gain access to functions guarded by concepts.

Why bother defining accessors like these:

inline double x_of(const Eta& e) {
    return e.x;
}
inline double y_of(const Eta& e) {
    return e.y;
}
inline double z_of(const Eta& e) {
    return e.z;
}

Just to get access to a simple function like this:

template <PositionLike T>
inline Eigen::Vector3d pos_vector(const T& t) {
    return Eigen::Vector3d{x_of(t), y_of(t), z_of(t)};
}

Well this design ensures better scalability when more types(structs) and functions are added.

It allows us to create general function like this, without increasing complexity too much:

template <PoseLike T>
geometry_msgs::msg::Pose pose_like_to_pose_msg(const T& pose_like) {
    geometry_msgs::msg::Pose pose;

    pose.position.x = x_of(pose_like);
    pose.position.y = y_of(pose_like);
    pose.position.z = z_of(pose_like);

    if constexpr (QuatPoseLike<T>) {
        pose.orientation.w = qw_of(pose_like);
        pose.orientation.x = qx_of(pose_like);
        pose.orientation.y = qy_of(pose_like);
        pose.orientation.z = qz_of(pose_like);
    } else if constexpr (EulerPoseLike<T>) {
        const auto q = vortex::utils::math::euler_to_quat(
            roll_of(pose_like), pitch_of(pose_like), yaw_of(pose_like));
        pose.orientation.w = q.w();
        pose.orientation.x = q.x();
        pose.orientation.y = q.y();
        pose.orientation.z = q.z();
    }

    return pose;
}

A downside is that we lose some explicitness. Most of the eta functions in ops.hpp could probably have been converted to more general purpose functions using the concepts, but at the loss of intent.

Thoughts?

@@ -0,0 +1,76 @@
#ifndef VORTEX_UTILS__ACCESSORS_HPP_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of creating functions to access publicly exposed members? Further, just curious, what are the specific usecases of all these generic functions other than possibly needing them in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The accessors are there so that arbitraty "pose" types can satisy the concept requirements. The generic functions like the ones in "views.hpp" can in the future, instead of having to define these functions for each new type, we just need to define the accessor functions and the generic functions will work.

/**
* @brief Eigen-based pose, for interfaces / measurements
*/
struct PoseQuatEigen {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why keep this and eta? They contain the same data just in different forms. The codebase needs to be refactored anyway to apply the other freefunctions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this does not need to be defined as is with eigen types. I am considering just creating a POD Pose struct, because Eta is not really a familiar term for perception

@jorgenfj
Copy link
Contributor Author

I reverted the Eta types back to their original structure. Also created a Pose struct that the perception boys can use. I don't know exactly how "domain" specific the eta structs are, if that is the case then having them more Object-oriented might make things more explicit and intent more clear. For general Pose structs giving them access the concept capabilities and the views functions should be sufficient for now. This setup IMO can achieve a good middle ground

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.14%. Comparing base (69391c1) to head (0ff4017).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #21      +/-   ##
==========================================
+ Coverage   98.11%   98.14%   +0.03%     
==========================================
  Files           5        6       +1     
  Lines         425      432       +7     
  Branches       97       84      -13     
==========================================
+ Hits          417      424       +7     
  Misses          4        4              
  Partials        4        4              
Flag Coverage Δ
unittests 98.14% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
include/vortex/utils/accessors.hpp 100.00% <100.00%> (ø)
include/vortex/utils/ros_conversions.hpp 100.00% <100.00%> (ø)
include/vortex/utils/types.hpp 95.58% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jorgenfj jorgenfj closed this Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants